-
Notifications
You must be signed in to change notification settings - Fork 0
[Mime type task] Update mime type solution #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
mime_type.rb
Outdated
| end | ||
|
|
||
| def match_file(filename) | ||
| splitted_fname = filename.split('.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not mix abbreviated variable names with full variable names. In Ruby, it's best to always use the full variable names.
mime_type.rb
Outdated
| splitted_fname = filename.split('.') | ||
| result = 'UNKNOWN' | ||
| if splitted_fname.size > 1 && filename[-1] != '.' | ||
| result = associations.fetch(splitted_fname.last.downcase, 'UNKNOWN') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can simplify the logic here:
You start with a result of 'UNKNOWN'. You check some condition and if it's true, you use Hash#fetch to either return a result, or provide the same default value of 'UNKNOWN'.
Try refactoring it in a way that would use 'UNKNOWN' only once
mime_type.rb
Outdated
|
|
||
| def start | ||
| q.times do | ||
| fname = gets.chomp # One file name per line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use full variable names
b8e5d88 to
73f6435
Compare
14f567d to
17b7168
Compare
Training/Easy/mime_type.rb
Outdated
| def match_file(filename) | ||
| unknown = 'UNKNOWN' | ||
| splitted_filename = filename.split('.') | ||
| if splitted_filename.size > 1 && filename[-1] != '.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's currently going to happen if that condition evaluates to false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vasil-gochev result is 'UNKNOWN'
Training/Easy/mime_type.rb
Outdated
| unknown = 'UNKNOWN' | ||
| splitted_filename = filename.split('.') | ||
| if splitted_filename.size > 1 && filename[-1] != '.' | ||
| result = associations.fetch(splitted_filename.last.downcase, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need Hash#fetch here at all - you can use Hash#[] instead
Training/Easy/mime_type.rb
Outdated
| if splitted_filename.size > 1 && filename[-1] != '.' | ||
| result = associations.fetch(splitted_filename.last.downcase, nil) | ||
| end | ||
| result = unknown if result.nil? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to check if the result is explicitly nil
Training/Easy/mime_type.rb
Outdated
| end | ||
|
|
||
| def start | ||
| q.times do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does q mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See line 7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know where to find it - I meant its name can be much more descriptive.
A clear indication of that is that you have to write comments to describe what your variables represent. Instead, you can just name them better and the comments won't be necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understood. This was left from the example code added on the task. I will fix it.
Training/Easy/mime_type.rb
Outdated
|
|
||
| def match_file(filename) | ||
| unknown = 'UNKNOWN' | ||
| splitted_filename = filename.split('.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you get when you split a filename?
No description provided.